Skip to content

revert(flows): restore require_approval default to true (merge LAST, after approval surfacing)#4672

Merged
graycyrus merged 1 commit into
tinyhumansai:mainfrom
graycyrus:feat/flows-restore-approval-defaults
Jul 7, 2026
Merged

revert(flows): restore require_approval default to true (merge LAST, after approval surfacing)#4672
graycyrus merged 1 commit into
tinyhumansai:mainfrom
graycyrus:feat/flows-restore-approval-defaults

Conversation

@graycyrus

@graycyrus graycyrus commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Restores require_approval's safe default of true by reverting the two band-aid PRs (#4651 + #4668), which had defaulted it to false purely to dodge the parked-run deadlock.

⚠️ MERGE ORDER: this lands LAST

Merge this only after the approval-surfacing PRs (run-details + chat + notifications, approve once/always) are in. Merging it before them reintroduces the deadlock (flows would default to require-approval again, but the approval still couldn't surface).

Changes (exact reverts of #4651 + #4668)

  • flows/tools.rs (ProposeWorkflowTool) — .unwrap_or(false).unwrap_or(true) + schema default
  • flows/builder_tools.rs (ReviseWorkflowTool) — same
  • WorkflowPromptBar.tsxcreateFlow(..., false)true
  • ChatRuntimeProvider.tsxrequire_approval === true!== false
  • tests reverted to the true default (explicit true/false round-trip tests kept)

Verified byte-for-byte against the pre-band-aid originals (25ce5b723, 8018eeb6e).

Verification

cargo check/fmt clean · flows:: green · typecheck clean · WorkflowPromptBar 5/5 · ChatRuntimeProvider 59/59.

Summary by CodeRabbit

  • Bug Fixes
    • Workflow creation now defaults to requiring approval for prompt-authored and agent-proposed flows.
    • Workflow proposal handling now treats omitted approval settings as enabled, matching the expected default behavior.
    • Updated related checks so approval settings are verified consistently across the app and backend.

…ansai#4651 + tinyhumansai#4668 band-aids)

tinyhumansai#4651 + tinyhumansai#4668 defaulted require_approval to false as a workaround for the parked-run
deadlock (a flow's approval couldn't surface with no thread/client). Now that the
approval-surfacing work makes flow approvals answerable (run-details + chat +
notifications, approve once/always), restore the SAFE default of true.

- flows/tools.rs (ProposeWorkflowTool): unwrap_or(false) -> unwrap_or(true) + schema default.
- flows/builder_tools.rs (ReviseWorkflowTool): same.
- WorkflowPromptBar.tsx: createFlow(..., false) -> true.
- ChatRuntimeProvider.tsx: require_approval === true -> !== false (default true).
- tests reverted to assert the true default; explicit true/false round-trip tests kept.

Byte-for-byte revert of the two band-aid diffs (verified against 25ce5b7 + 8018eeb).
cargo check/fmt/flows:: green; typecheck + WorkflowPromptBar 5/5 + ChatRuntimeProvider 59/59.

MUST MERGE LAST — after the approval-surfacing PRs, or it reintroduces the deadlock.
@graycyrus graycyrus requested a review from a team July 7, 2026 20:47
@graycyrus graycyrus marked this pull request as draft July 7, 2026 20:47
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR flips the default value of require_approval from false to true when the field is omitted, across the Rust ProposeWorkflowTool and revise_workflow tool, the frontend ChatRuntimeProvider proposal parser, and WorkflowPromptBar's flow creation call, with corresponding test updates.

Changes

require_approval default flip

Layer / File(s) Summary
ProposeWorkflowTool defaulting
src/openhuman/flows/tools.rs, src/openhuman/flows/tools_tests.rs
Schema description and unwrap_or default for require_approval changed from false to true; test renamed and assertion flipped.
revise_workflow tool defaulting
src/openhuman/flows/builder_tools.rs, src/openhuman/flows/builder_tools_tests.rs
Schema description and execution default for require_approval changed from false to true; test renamed and assertion flipped.
Frontend proposal parsing and prompt bar creation
app/src/providers/ChatRuntimeProvider.tsx, app/src/components/flows/WorkflowPromptBar.tsx, app/src/components/flows/WorkflowPromptBar.test.tsx
parseWorkflowProposal now treats missing/undefined require_approval as true (only explicit false yields false); WorkflowPromptBar now passes true for requireApproval when calling createFlow; tests updated to match.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: bug

Poem

A flag once false, now flipped to true,
Approval waits on flows anew.
In Rust and React, hop in sync,
No more silent skip, just think—
🐰 A rabbit cheers this safer view!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: restoring the flow approval default to true, with only minor extra merge-order context.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/openhuman/flows/tools.rs (1)

141-245: 🧹 Nitpick | 🔵 Trivial

Default flip is consistent, but confirm merge sequencing.

The unwrap_or(true) change and the resulting require_approval echoed in the response (Line 241) are internally consistent with the schema description and with revise_workflow's matching change in builder_tools.rs. Per the PR stack context, this revert is intended to be merged last, after the approval-surfacing work is in place, to avoid reintroducing the parked-run deadlock for flows awaiting approval. Please confirm that dependency has actually landed before merging this cohort.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/flows/tools.rs` around lines 141 - 245, The `execute` flow in
`propose_workflow` now flips `require_approval` to default true and returns it
in the proposal payload, so make sure the approval-surfacing changes it depends
on are already merged before this lands. Verify the matching logic in
`builder_tools.rs` and the surrounding workflow approval path is present so the
default flip does not reintroduce the parked-run deadlock for pending flows.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/openhuman/flows/tools.rs`:
- Around line 141-245: The `execute` flow in `propose_workflow` now flips
`require_approval` to default true and returns it in the proposal payload, so
make sure the approval-surfacing changes it depends on are already merged before
this lands. Verify the matching logic in `builder_tools.rs` and the surrounding
workflow approval path is present so the default flip does not reintroduce the
parked-run deadlock for pending flows.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eabea221-f0b1-459c-9c05-1ef2ce3d4bc6

📥 Commits

Reviewing files that changed from the base of the PR and between c0e7b51 and c26c39f.

📒 Files selected for processing (7)
  • app/src/components/flows/WorkflowPromptBar.test.tsx
  • app/src/components/flows/WorkflowPromptBar.tsx
  • app/src/providers/ChatRuntimeProvider.tsx
  • src/openhuman/flows/builder_tools.rs
  • src/openhuman/flows/builder_tools_tests.rs
  • src/openhuman/flows/tools.rs
  • src/openhuman/flows/tools_tests.rs

@graycyrus graycyrus marked this pull request as ready for review July 7, 2026 21:21
@graycyrus graycyrus merged commit 9168365 into tinyhumansai:main Jul 7, 2026
19 of 23 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Team Openhuman Jul 7, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c26c39f74d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

name,
createBlankWorkflowGraph(name, t('flows.nodeKind.trigger')),
false
true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid parking prompt-authored flows on an unsurfaced gate

With this default, every prompt-created flow saved by the builder carries require_approval=true, so a Run-button or scheduled run that reaches an outbound HTTP/tool/code node enters ApprovalGate's workflow branch. That branch explicitly publishes a generic ApprovalRequested with no chat routing, while the current UI still does not route approval attention actions (TinyPlaceOrchestrationTab only handles open-session; FlowApprovalCard calls flows_resume, not approval_decide). In those non-chat runs the tool call therefore waits until the gate TTL and fails instead of executing, which regresses the default prompt-authoring path for any real outbound automation.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant